Skip to content

Fix #4288 vschema errors should not bring down vtgate#4371

Merged
sougou merged 2 commits intovitessio:masterfrom
planetscale:ds-vschema-error
Nov 19, 2018
Merged

Fix #4288 vschema errors should not bring down vtgate#4371
sougou merged 2 commits intovitessio:masterfrom
planetscale:ds-vschema-error

Conversation

@deepthi
Copy link
Copy Markdown
Collaborator

@deepthi deepthi commented Nov 17, 2018

Save errors per keyspace in vschema instead of failing on the first
error so that keyspaces with valid vschema don't get invalidated by a
keyspace with vschema errors

Signed-off-by: deepthi deepthi@planetscale.com

Save errors per keyspace in vschema instead of failing on the first
error so that keyspaces with valid vschema don't get invalidated by a
keyspace with vschema errors

Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi requested a review from sougou as a code owner November 17, 2018 01:00
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Couple of nits.

Sharded: ks.Keyspace.Sharded,
Tables: ks.Tables,
Vindexes: ks.Vindexes,
Error: func(ks *KeyspaceSchema) string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ok here, but should be used with care because inline functions incur additional overhead.

if err != nil {
return fmt.Errorf("could not decode the keyspace id for pin: %v", err)
ksvschema.Error = fmt.Errorf("could not decode the keyspace id for pin: %v", err)
goto end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, you can tag the outer loop like this:

outer:
  for ksname...

and then you can continue outer.

Signed-off-by: deepthi <deepthi@planetscale.com>
@sougou sougou merged commit b0d2eee into vitessio:master Nov 19, 2018
@deepthi deepthi deleted the ds-vschema-error branch December 5, 2018 01:09
dweitzman added a commit to dweitzman/vitess that referenced this pull request Mar 22, 2019
…er sequences

PR vitessio#4371 made vtgate more permissive about allowing bad data in a vschema
without failing requests that only involve properly-configured tables.

This PR extends that so a single table entry in the vschema that references a bad
sequence won't disable the sequences for all the other tables with valid sequence
configs.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants